Handle ringColor.DEFAULT in js configs
#19348
Conversation
WalkthroughThis pull request adds support for handling Pre-merge checks✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/@tailwindcss-upgrade/src/codemods/config/migrate-js-config.ts (1)
240-246: Migration ofringColor.DEFAULTto--default-ring-colorlooks correctThe upgrade path now emits
--default-ring-colorforringColor.DEFAULTand skips the generickeyPathToCssPropertymapping, keeping the generated CSS consistent with the runtime theme behavior and avoiding an incorrect--ring-colorvariable.If we add more special-cased theme keys in future, consider centralizing this mapping so
apply-config-to-themeand the upgrader can’t drift apart.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)integrations/upgrade/js-config.test.ts(2 hunks)packages/@tailwindcss-upgrade/src/codemods/config/migrate-js-config.ts(1 hunks)packages/tailwindcss/src/compat/apply-config-to-theme.test.ts(2 hunks)packages/tailwindcss/src/compat/apply-config-to-theme.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/@tailwindcss-upgrade/src/codemods/config/migrate-js-config.ts (1)
packages/tailwindcss/src/utils/escape.ts (1)
escape(2-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Linux / upgrade
- GitHub Check: Linux / vite
- GitHub Check: Linux / postcss
- GitHub Check: Linux
🔇 Additional comments (5)
CHANGELOG.md (1)
18-18: Changelog entry correctly describes theringColor.DEFAULTfixThe new “Fixed” bullet is accurate, scoped, and links to the correct PR, so the release notes will clearly communicate this behavior change.
packages/tailwindcss/src/compat/apply-config-to-theme.ts (1)
53-63: Special-case forringColor.DEFAULT→--default-ring-coloris correct and consistentHandling
ringColor.DEFAULTbefore the generickeyPathToCssPropertypath ensures we no longer emit--ring-colorand instead populate--default-ring-colorwith the right theme options, matching the intended v3-compatible behavior.packages/tailwindcss/src/compat/apply-config-to-theme.test.ts (1)
60-63: Tests accurately cover the new default ring color behaviorAdding
ringColor.DEFAULTto the test config and asserting that--ring-coloris unset while--default-ring-coloris'#fff'validates the new mapping end-to-end and guards against regressions.Also applies to: 132-133
integrations/upgrade/js-config.test.ts (2)
37-39: LGTM! Test input properly configured.The
ringColor.DEFAULTconfiguration correctly sets up the test case for validating the v3 to v4 migration behavior. The test value#c0ffeeis clearly identifiable in the output.
197-198: LGTM! Expected output correctly validates the migration.The test properly verifies that
ringColor.DEFAULTmigrates to--default-ring-color(not--ring-color), which aligns with the PR objectives. The snapshot-based assertion implicitly confirms that the old--ring-colorvariable does not appear in the output.
Fixes #19345
In v3 the
ringColor.DEFAULToption was used as the default color for ring utilities (when it was defined). This currently gets translated as--ring-colorbut that doesn't work this way in v4. Instead it should translate to--default-ring-colorand not--ring-color.I've also tweaked the upgrade tool to handle this properly as well.